Skip to content

Disable a few EC curves in computing shared secret #826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Disable a few EC curves in computing shared secret #826

wants to merge 1 commit into from
+148 −42

Conversation

@JinhangZhang
Copy link
Collaborator

@JinhangZhang JinhangZhang commented Oct 3, 2025 •
edited
Loading

According to the policy, curve size less than 192 should not be allowed in computing shared secret in ECDH keyagreement in FIPS 140-3 mode.

@JinhangZhang JinhangZhang force-pushed the openjceplus/bug/ecdhkeyagreementp192curve branch 3 times, most recently from 142a25e to 658cc4d Compare October 25, 2025 01:17
@JinhangZhang JinhangZhang marked this pull request as ready for review October 25, 2025 01:17
jasonkatonica
jasonkatonica requested changes Oct 27, 2025
View reviewed changes
@jasonkatonica jasonkatonica self-requested a review October 27, 2025 13:33
jasonkatonica
jasonkatonica requested changes Oct 27, 2025
View reviewed changes
Copy link
Member

@jasonkatonica jasonkatonica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also various checkstyle failures:

Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:12:1: Extra separation in import group before 'org.junit.jupiter.api.Assertions.assertTrue' [ImportOrder]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:14:1: Extra separation in import group before 'java.security.KeyPair' [ImportOrder]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:14:1: Wrong order for 'java.security.KeyPair' import. [ImportOrder]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:18:8: Unused import - java.security.SecureRandom. [UnusedImports]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:21:1: Extra separation in import group before 'javax.crypto.KeyAgreement' [ImportOrder]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:23:1: Extra separation in import group before 'org.junit.jupiter.api.BeforeAll' [ImportOrder]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:24:8: Unused import - org.junit.jupiter.api.Test. [UnusedImports]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:46:13: Unused local variable 'secretAlice'. [UnusedLocalVariable]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:47:13: Unused local variable 'secretBob'. [UnusedLocalVariable]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/main/java/com/ibm/crypto/plus/provider/ECDHKeyAgreement.java:48:163: ',' is not followed by whitespace. [WhitespaceAfter]

@JinhangZhang
Copy link
Collaborator Author

JinhangZhang commented Oct 27, 2025

There are also various checkstyle failures:

Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:12:1: Extra separation in import group before 'org.junit.jupiter.api.Assertions.assertTrue' [ImportOrder]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:14:1: Extra separation in import group before 'java.security.KeyPair' [ImportOrder]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:14:1: Wrong order for 'java.security.KeyPair' import. [ImportOrder]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:18:8: Unused import - java.security.SecureRandom. [UnusedImports]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:21:1: Extra separation in import group before 'javax.crypto.KeyAgreement' [ImportOrder]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:23:1: Extra separation in import group before 'org.junit.jupiter.api.BeforeAll' [ImportOrder]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:24:8: Unused import - org.junit.jupiter.api.Test. [UnusedImports]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:46:13: Unused local variable 'secretAlice'. [UnusedLocalVariable]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/test/java/ibm/jceplus/junit/openjceplusfips/TestECDHKeyAgreementParamValidation.java:47:13: Unused local variable 'secretBob'. [UnusedLocalVariable]
Error:  /home/runner/work/OpenJCEPlus/OpenJCEPlus/src/main/java/com/ibm/crypto/plus/provider/ECDHKeyAgreement.java:48:163: ',' is not followed by whitespace. [WhitespaceAfter]

Updated

jasonkatonica
jasonkatonica requested changes Oct 27, 2025
View reviewed changes
KostasTsiounis
KostasTsiounis requested changes Oct 27, 2025
View reviewed changes
@JinhangZhang JinhangZhang force-pushed the openjceplus/bug/ecdhkeyagreementp192curve branch 2 times, most recently from f34f89b to a1034a7 Compare October 28, 2025 05:43
@JinhangZhang JinhangZhang force-pushed the openjceplus/bug/ecdhkeyagreementp192curve branch from a1034a7 to d420201 Compare October 28, 2025 14:12
@JinhangZhang JinhangZhang force-pushed the openjceplus/bug/ecdhkeyagreementp192curve branch from d420201 to a53dbf9 Compare October 28, 2025 17:26
jasonkatonica
jasonkatonica requested changes Oct 29, 2025
View reviewed changes
Copy link
Member

@jasonkatonica jasonkatonica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some tests are currently failing with the latest code here for example:

[2025-10-29T13:44:41.910Z] java.lang.IllegalStateException: secp256k1 curve is not supported in FIPS for public key

[2025-10-29T13:44:41.910Z] 	at openjceplus/com.ibm.crypto.plus.provider.ECDHKeyAgreement.engineGenerateSecret(ECDHKeyAgreement.java:186)

[2025-10-29T13:44:41.910Z] 	at java.base/javax.crypto.KeyAgreement.generateSecret(KeyAgreement.java:613)

[2025-10-29T13:44:41.910Z] 	at openjceplus/ibm.jceplus.junit.base.BaseTestECDH.compute_ecdh_key(BaseTestECDH.java:268)

[2025-10-29T13:44:41.910Z] 	at openjceplus/ibm.jceplus.junit.base.BaseTestECDH.testECDH_ECSpec(BaseTestECDH.java:157)

[2025-10-29T13:44:41.910Z] 	at java.base/java.lang.reflect.Method.invoke(Method.java:571)

[2025-10-29T13:44:41.910Z] 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)

[2025-10-29T13:44:41.910Z] 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)

2025-10-29T13:44:42.457Z] [ERROR] Failures: 

[2025-10-29T13:44:42.457Z] [ERROR]   TestMultithreadFIPS.testMultithreadFIPS:178 Failed tests:

[2025-10-29T13:44:42.457Z] 	ibm.jceplus.junit.openjceplusfips.multithread.TestECDH

[2025-10-29T13:44:42.457Z] [ERROR] Errors: 

[2025-10-29T13:44:42.457Z] [ERROR]   TestECDH>BaseTestECDH.testECDH_ECSpec:157->BaseTestECDH.compute_ecdh_key:268 � IllegalState secp256k1 curve is not supported in FIPS for public key

[2025-10-29T13:44:42.457Z] [ERROR]   TestECDH>BaseTestECDH.testECDH_secp192k1:114->BaseTestECDH.compute_ecdh_key:268 � IllegalState secp192k1 curve is not supported in FIPS for public key

[2025-10-29T13:44:42.457Z] [INFO] 

[2025-10-29T13:44:42.457Z] [ERROR] Tests run: 2936, Failures: 1, Errors: 2, Skipped: 11

@JinhangZhang
Copy link
Collaborator Author

JinhangZhang commented Oct 29, 2025

Some tests are currently failing with the latest code here for example:

[2025-10-29T13:44:41.910Z] java.lang.IllegalStateException: secp256k1 curve is not supported in FIPS for public key

[2025-10-29T13:44:41.910Z] 	at openjceplus/com.ibm.crypto.plus.provider.ECDHKeyAgreement.engineGenerateSecret(ECDHKeyAgreement.java:186)

[2025-10-29T13:44:41.910Z] 	at java.base/javax.crypto.KeyAgreement.generateSecret(KeyAgreement.java:613)

[2025-10-29T13:44:41.910Z] 	at openjceplus/ibm.jceplus.junit.base.BaseTestECDH.compute_ecdh_key(BaseTestECDH.java:268)

[2025-10-29T13:44:41.910Z] 	at openjceplus/ibm.jceplus.junit.base.BaseTestECDH.testECDH_ECSpec(BaseTestECDH.java:157)

[2025-10-29T13:44:41.910Z] 	at java.base/java.lang.reflect.Method.invoke(Method.java:571)

[2025-10-29T13:44:41.910Z] 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)

[2025-10-29T13:44:41.910Z] 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)

2025-10-29T13:44:42.457Z] [ERROR] Failures: 

[2025-10-29T13:44:42.457Z] [ERROR]   TestMultithreadFIPS.testMultithreadFIPS:178 Failed tests:

[2025-10-29T13:44:42.457Z] 	ibm.jceplus.junit.openjceplusfips.multithread.TestECDH

[2025-10-29T13:44:42.457Z] [ERROR] Errors: 

[2025-10-29T13:44:42.457Z] [ERROR]   TestECDH>BaseTestECDH.testECDH_ECSpec:157->BaseTestECDH.compute_ecdh_key:268 � IllegalState secp256k1 curve is not supported in FIPS for public key

[2025-10-29T13:44:42.457Z] [ERROR]   TestECDH>BaseTestECDH.testECDH_secp192k1:114->BaseTestECDH.compute_ecdh_key:268 � IllegalState secp192k1 curve is not supported in FIPS for public key

[2025-10-29T13:44:42.457Z] [INFO] 

[2025-10-29T13:44:42.457Z] [ERROR] Tests run: 2936, Failures: 1, Errors: 2, Skipped: 11

When I tested and printed the EC curves supported in FIPS 140-3, I got the following:

OpenJCEPlusFIPS
1.3.132.0.35 = true
1.3.132.0.9 = false
1.3.132.0.33 = true
1.3.132.0.8 = false
1.3.132.0.34 = true
1.3.36.3.3.2.8.1.1.9 = false
1.3.132.0.7 = false
1.3.132.0.31 = false
1.3.132.0.6 = false
1.3.132.0.32 = false
1.3.132.0.10 = false
1.3.36.3.3.2.8.1.1.7 = false
1.3.132.0.30 = false
1.3.36.3.3.2.8.1.1.5 = false
1.2.840.10045.3.1.1 = true
1.2.840.10045.3.1.2 = false
1.3.36.3.3.2.8.1.1.3 = false
1.3.36.3.3.2.8.1.1.1 = false
1.2.840.10045.3.1.5 = false
1.2.840.10045.3.1.6 = false
1.2.840.10045.3.1.3 = false
1.2.840.10045.3.1.4 = false
1.3.36.3.3.2.8.1.1.11 = false
1.3.36.3.3.2.8.1.1.13 = false
1.3.132.0.28 = false
1.3.132.0.29 = false
1.2.840.10045.3.1.7 = true
1.3.132.0.35 = true
1.3.132.0.9 = false
1.3.132.0.33 = true
1.3.132.0.8 = false
1.3.132.0.34 = true
1.3.36.3.3.2.8.1.1.9 = false
1.3.132.0.7 = false
1.3.132.0.31 = false
1.3.132.0.6 = false
1.3.132.0.32 = false
1.3.132.0.10 = false
1.3.36.3.3.2.8.1.1.7 = false
1.3.132.0.30 = false
1.3.36.3.3.2.8.1.1.5 = false
1.2.840.10045.3.1.1 = true
1.2.840.10045.3.1.2 = false
1.3.36.3.3.2.8.1.1.3 = false
1.3.36.3.3.2.8.1.1.1 = false
1.2.840.10045.3.1.5 = false
1.2.840.10045.3.1.6 = false
1.2.840.10045.3.1.3 = false
1.2.840.10045.3.1.4 = false
1.3.36.3.3.2.8.1.1.11 = false
1.3.36.3.3.2.8.1.1.13 = false
1.3.132.0.28 = false
1.3.132.0.29 = false
1.2.840.10045.3.1.7 = true

secp256k1's oid is 1.3.132.0.10 which is not supported in FIPS140-3.

@JinhangZhang JinhangZhang force-pushed the openjceplus/bug/ecdhkeyagreementp192curve branch 5 times, most recently from 79e8788 to 0acfd74 Compare October 30, 2025 15:14
@JinhangZhang JinhangZhang force-pushed the openjceplus/bug/ecdhkeyagreementp192curve branch from 0acfd74 to 50d2ecb Compare October 30, 2025 19:53
KostasTsiounis
KostasTsiounis requested changes Oct 31, 2025
View reviewed changes
@JinhangZhang JinhangZhang force-pushed the openjceplus/bug/ecdhkeyagreementp192curve branch from 50d2ecb to a9e6df6 Compare November 3, 2025 18:50
KostasTsiounis
KostasTsiounis approved these changes Nov 3, 2025
View reviewed changes
Copy link
Member

@KostasTsiounis KostasTsiounis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jasonkatonica
jasonkatonica requested changes Nov 4, 2025
View reviewed changes
@JinhangZhang JinhangZhang mentioned this pull request Nov 4, 2025
Open
jasonkatonica
jasonkatonica requested changes Nov 4, 2025
View reviewed changes
@JinhangZhang
Restrict EC curves in computing shared secret
ad3d5fe 
According to the policy, curve size less than 192 should
not be allowed in computing shared secret in ECDH
keyagreement in FIPS 140-3 mode. This PR proposes an option
to enable/disable this behaviour.

Signed-off-by: JinhangZhang <[email protected]>
@JinhangZhang JinhangZhang force-pushed the openjceplus/bug/ecdhkeyagreementp192curve branch from a9e6df6 to ad3d5fe Compare November 5, 2025 16:43
jasonkatonica
jasonkatonica approved these changes Nov 5, 2025
View reviewed changes
Copy link
Member

@jasonkatonica jasonkatonica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

johnpeck-us-ibm
johnpeck-us-ibm approved these changes Nov 6, 2025
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KostasTsiounis KostasTsiounis KostasTsiounis approved these changes

@jasonkatonica jasonkatonica jasonkatonica approved these changes

@johnpeck-us-ibm johnpeck-us-ibm johnpeck-us-ibm approved these changes

@taoliult taoliult Awaiting requested review from taoliult

Assignees

@JinhangZhang JinhangZhang

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JinhangZhang @KostasTsiounis @jasonkatonica @johnpeck-us-ibm